From 640487c9e8879e543713b5941af60bb01ac0a2a6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 23 Oct 2014 13:04:08 -0700 Subject: [PATCH] Avoid updating the registry too frequently This logic is based off the precision of the registry source's id as well as the dependencies being passed in. More info can be found in the comments. --- src/cargo/ops/cargo_generate_lockfile.rs | 5 +- src/cargo/ops/resolve.rs | 2 +- src/cargo/sources/registry.rs | 52 ++++++++++++---- tests/test_cargo_registry.rs | 76 ++++++++++++++++++++---- 4 files changed, 109 insertions(+), 26 deletions(-) diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index c318b8db9..059e9064c 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -49,7 +49,6 @@ pub fn update_lockfile(manifest_path: &Path, let mut config = try!(Config::new(opts.shell, None, None)); let mut registry = PackageRegistry::new(&mut config); let mut to_avoid = HashSet::new(); - let mut previous = Some(&previous_resolve); match opts.to_update { Some(name) => { @@ -69,13 +68,13 @@ pub fn update_lockfile(manifest_path: &Path, } } } - None => { previous = None; } + None => to_avoid.extend(previous_resolve.iter()), } let resolve = try!(ops::resolve_with_previous(&mut registry, &package, resolver::ResolveEverything, - previous, + Some(&previous_resolve), Some(&to_avoid))); try!(ops::write_pkg_lockfile(&package, &resolve)); return Ok(()); diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 7f2a9fd21..62b6e9f3f 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -87,7 +87,7 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry, let mut resolved = try!(resolver::resolve(&summary, method, registry)); match previous { - Some(r) => resolved.copy_metadata(previous), + Some(r) => resolved.copy_metadata(r), None => {} } return Ok(resolved); diff --git a/src/cargo/sources/registry.rs b/src/cargo/sources/registry.rs index ad21312be..f989559c3 100644 --- a/src/cargo/sources/registry.rs +++ b/src/cargo/sources/registry.rs @@ -189,6 +189,7 @@ pub struct RegistrySource<'a, 'b:'a> { sources: Vec, hashes: HashMap<(String, String), String>, // (name, vers) => cksum cache: HashMap>, + updated: bool, } #[deriving(Decodable)] @@ -239,6 +240,7 @@ impl<'a, 'b> RegistrySource<'a, 'b> { sources: Vec::new(), hashes: HashMap::new(), cache: HashMap::new(), + updated: false, } } @@ -420,20 +422,11 @@ impl<'a, 'b> RegistrySource<'a, 'b> { .default_features(default_features) .features(features)) } -} -impl<'a, 'b> Registry for RegistrySource<'a, 'b> { - fn query(&mut self, dep: &Dependency) -> CargoResult> { - let summaries = try!(self.summaries(dep.get_name())); - let mut summaries = summaries.iter().filter(|&&(_, yanked)| { - dep.get_source_id().get_precise().is_some() || !yanked - }).map(|&(ref s, _)| s.clone()).collect::>(); - summaries.query(dep) - } -} + /// Actually perform network operations to update the registry + fn do_update(&mut self) -> CargoResult<()> { + if self.updated { return Ok(()) } -impl<'a, 'b> Source for RegistrySource<'a, 'b> { - fn update(&mut self) -> CargoResult<()> { try!(self.config.shell().status("Updating", format!("registry `{}`", self.source_id.get_url()))); let repo = try!(self.open()); @@ -451,6 +444,41 @@ impl<'a, 'b> Source for RegistrySource<'a, 'b> { log!(5, "[{}] updating to rev {}", self.source_id, oid); let object = try!(repo.find_object(oid, None)); try!(repo.reset(&object, git2::Hard, None, None)); + self.updated = true; + self.cache.clear(); + Ok(()) + } +} + +impl<'a, 'b> Registry for RegistrySource<'a, 'b> { + fn query(&mut self, dep: &Dependency) -> CargoResult> { + // If this is a precise dependency, then it came from a lockfile and in + // theory the registry is known to contain this version. If, however, we + // come back with no summaries, then our registry may need to be + // updated, so we fall back to performing a lazy update. + if dep.get_source_id().get_precise().is_some() && + try!(self.summaries(dep.get_name())).len() == 0 { + try!(self.do_update()); + } + + let summaries = try!(self.summaries(dep.get_name())); + let mut summaries = summaries.iter().filter(|&&(_, yanked)| { + dep.get_source_id().get_precise().is_some() || !yanked + }).map(|&(ref s, _)| s.clone()).collect::>(); + summaries.query(dep) + } +} + +impl<'a, 'b> Source for RegistrySource<'a, 'b> { + fn update(&mut self) -> CargoResult<()> { + // If we have an imprecise version then we don't know what we're going + // to look for, so we always atempt to perform an update here. + // + // If we have a precise version, then we'll update lazily during the + // querying phase. + if self.source_id.get_precise().is_none() { + try!(self.do_update()); + } Ok(()) } diff --git a/tests/test_cargo_registry.rs b/tests/test_cargo_registry.rs index 27e34d669..596764b26 100644 --- a/tests/test_cargo_registry.rs +++ b/tests/test_cargo_registry.rs @@ -2,7 +2,7 @@ use std::io::{fs, File}; use support::{project, execs, cargo_dir}; use support::{UPDATING, DOWNLOADING, COMPILING, PACKAGING, VERIFYING}; -use support::paths::PathExt; +use support::paths::{mod, PathExt}; use support::registry as r; use hamcrest::assert_that; @@ -249,9 +249,7 @@ test!(lockfile_locks { r::mock_pkg("bar", "0.0.2", []); assert_that(p.process(cargo_dir().join("cargo")).arg("build"), - execs().with_status(0).with_stdout(format!("\ -{updating} registry `[..]` -", updating = UPDATING).as_slice())); + execs().with_status(0).with_stdout("")); }) test!(lockfile_locks_transitively { @@ -287,9 +285,7 @@ test!(lockfile_locks_transitively { r::mock_pkg("bar", "0.0.2", [("baz", "*")]); assert_that(p.process(cargo_dir().join("cargo")).arg("build"), - execs().with_status(0).with_stdout(format!("\ -{updating} registry `[..]` -", updating = UPDATING).as_slice())); + execs().with_status(0).with_stdout("")); }) test!(yanks_are_not_used { @@ -373,9 +369,7 @@ test!(yanks_in_lockfiles_are_ok { r::mock_pkg_yank("bar", "0.0.1", [], true); assert_that(p.process(cargo_dir().join("cargo")).arg("build"), - execs().with_status(0).with_stdout(format!("\ -{updating} registry `[..]` -", updating = UPDATING).as_slice())); + execs().with_status(0).with_stdout("")); assert_that(p.process(cargo_dir().join("cargo")).arg("update"), execs().with_status(101).with_stderr("\ @@ -384,3 +378,65 @@ location searched: the package registry version required: * ")); }) + +test!(update_with_lockfile_if_packages_missing { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "*" + "#) + .file("src/main.rs", "fn main() {}"); + p.build(); + + r::mock_pkg("bar", "0.0.1", []); + assert_that(p.process(cargo_dir().join("cargo")).arg("build"), + execs().with_status(0)); + p.root().move_into_the_past().unwrap(); + + fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap(); + assert_that(p.process(cargo_dir().join("cargo")).arg("build"), + execs().with_status(0).with_stdout(format!("\ +{updating} registry `[..]` +{downloading} bar v0.0.1 (the package registry) +", updating = UPDATING, downloading = DOWNLOADING).as_slice())); +}) + +test!(update_lockfile { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = "*" + "#) + .file("src/main.rs", "fn main() {}"); + p.build(); + + r::mock_pkg("bar", "0.0.1", []); + assert_that(p.process(cargo_dir().join("cargo")).arg("build"), + execs().with_status(0)); + + r::mock_pkg("bar", "0.0.2", []); + fs::rmdir_recursive(&paths::home().join(".cargo/registry")).unwrap(); + assert_that(p.process(cargo_dir().join("cargo")).arg("update") + .arg("-p").arg("bar"), + execs().with_status(0).with_stdout(format!("\ +{updating} registry `[..]` +", updating = UPDATING).as_slice())); + + assert_that(p.process(cargo_dir().join("cargo")).arg("build"), + execs().with_status(0).with_stdout(format!("\ +{downloading} [..] v0.0.2 (the package registry) +{compiling} bar v0.0.2 (the package registry) +{compiling} foo v0.0.1 ({dir}) +", downloading = DOWNLOADING, compiling = COMPILING, + dir = p.url()).as_slice())); +}) -- 2.30.2